8380060: C2: Wrong execution with COH and arraycopy#30873
8380060: C2: Wrong execution with COH and arraycopy#30873rwestrel wants to merge 11 commits intoopenjdk:masterfrom
Conversation
…hape of AC memory
|
👋 Welcome back roland! A progress list of the required criteria for merging this PR into |
|
/contributor add chagedorn |
|
/contributor add Luca Ardueser, BSI Software AG |
|
❗ This change is not yet ready to be integrated. |
|
/contributor add @rkennke |
|
@rwestrel |
|
@rwestrel Syntax:
User names can only be used for users in the census associated with this repository. For other contributors you need to supply the full name and email address. |
|
@rwestrel |
|
The total number of required reviews for this PR has been set to 2 based on the presence of this label: |
|
/touch |
|
@rwestrel The pull request is being re-evaluated and the inactivity timeout has been reset. |
There was a problem hiding this comment.
Whoa-whoa... so if get_alias_index has a side-effect of allocating new index, does that mean that all uses of get_alias_index that are wrapped in DEBUG_ONLY or ASSERT are risky and need to be audited?
Because it is scary if they can breaking release builds without showing up in debug builds. Like this one.
shipilev
left a comment
shipilev
left a comment
There was a problem hiding this comment.
But the local fix does look good.
The ones in EA are the risky ones, I think, because we add new indexes there. |
| if (adr_type != new_adr_type && !init->already_has_narrow_mem_proj_with_adr_type(new_adr_type)) { | ||
| DEBUG_ONLY( uint alias_idx = _compile->get_alias_index(new_adr_type); ) | ||
| // Do NOT remove the next line: ensure a new alias index is allocated for the instance type. | ||
| uint alias_idx = _compile->get_alias_index(new_adr_type); |
There was a problem hiding this comment.
It makes sense to place alias index probing into NarrowMemProjNode ctor. Or, at least, put an assert there to ensure the index is present.
4 participants
This was discussed in:
#30775
get_alias_index()has the side effect of allocating a new alias so if we don't perform the call then code that iterates over all aliases untilnum_alias_types()to fix the memory graph during EA skips the newly addedNarrowMemProjNode.Progress
Issue
Reviewers
Contributors
<chagedorn@openjdk.org><rkennke@openjdk.org>Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/30873/head:pull/30873$ git checkout pull/30873Update a local copy of the PR:
$ git checkout pull/30873$ git pull https://git.openjdk.org/jdk.git pull/30873/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 30873View PR using the GUI difftool:
$ git pr show -t 30873Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/30873.diff
Using Webrev
Link to Webrev Comment